Skip to content

feat: port the React Compiler's HIR + validators into react-doctor#164

Draft
aidenybai wants to merge 16 commits intomainfrom
cursor/hir-port-7144
Draft

feat: port the React Compiler's HIR + validators into react-doctor#164
aidenybai wants to merge 16 commits intomainfrom
cursor/hir-port-7144

Conversation

@aidenybai
Copy link
Copy Markdown
Member

@aidenybai aidenybai commented May 8, 2026

Summary

Ports the React Compiler's High-level IR (HIR) infrastructure into react-doctor as a foundation for more precise rule analysis, plus two initial validators ported from the compiler:

  • hir-no-set-state-in-effect: flags setState calls inside useEffect bodies, including when the setter is propagated through const x = setX aliasing or wrapped in a useEffectEvent / inner FunctionExpression.
  • hir-no-derived-computations-in-effects: flags effects whose captures are entirely deps + setters and whose body has at least one intermediate const (the unique path the AST walker can't see through).

What got ported

  • HIR data model (plugin/hir/types.ts): HIRFunction, BasicBlock, Instruction, Place, Identifier, ReactType (≈20 React-aware tags including the round-5 StateTuple), with originNode on Place for diagnostic precision.
  • Lowering (plugin/hir/lower.ts): ESTree → HIR, parent-chained scope environments, FunctionExpression.capturedPlaces, full statement coverage (round-5 added for/while/do-while/for-of/for-in/switch/try/throw/labeled), SpreadElement unwrapping in call args.
  • Type inference (plugin/hir/infer-types.ts): hook recognition via LoadGlobal name → ReactType, propagation through LoadLocal/StoreLocal/PropertyLoad. Round-5 introduces StateTuple so only useState returns get the indexed StateValue/StateSetter tagging — useMemo/useContext returns no longer leak into that branch.
  • Validators (plugin/hir/validators/): two ports, with shape-defer logic that lets the existing AST walker handle the simple cases.
  • Runner (plugin/hir/runner.ts): per-component WeakMap<EsTreeNode, HIRFunction> cache + originNode-based diagnostic anchoring so reports point at the offending setX(...) call site.

Round 5 — deep code review fixes

Five issues found on a fresh review of the round-4 code:

# Issue Fix
1 inferTypes conflated useState/useMemo/useContext returns as Object; the indexed-PropertyLoad branch tagged [a, b] = useMemo(...) as [StateValue, StateSetter], leading to false-positive setState detection New StateTuple ReactType gates the indexed-PropertyLoad branch to useState only; regression test for useMemo tuple destructure
2 hir-no-derived-computations-in-effects duplicated noDerivedStateEffect on the article §1 fullName example Validator now defers when the inner HIR has no StoreLocal (single-setter-call shape) — keeps the multi-statement-with-locals path that uniquely needs HIR. hir-no-set-state-in-effect left at warn with a known-overlap note in oxlint-config.ts, since reliably distinguishing direct vs aliased setters at v1 isn't workable
3 lowerExpression silently dropped SpreadElement arguments New lowerCallArguments unwraps the spread's argument so operand identity is preserved through setState propagation
4 lowerStatement ignored for/while/do-while/for-of/for-in/switch/try/throw/labeled — hooks inside any of these blocks were invisible Added recursive descent for all of them
5 hir-unit.test.ts article-§1 case used the single-setter-call shape that fix #2 now defers on Updated to the multi-statement-with-locals form to exercise the validator's unique path

Validation

  • 490 / 490 tests pass (4 new regressions added across the 5 fixes)
  • pnpm lint clean
  • pnpm typecheck clean
  • pnpm format clean

Known v1 limitations (documented in code)

  • let reassignments are not modeled as non-reactive (HIR is non-SSA).
  • JSX expression contents aren't lowered as identifier reads (renders are summarized as a placeholder).
  • import { useState as useS } is not recognized — name-based heuristic on LoadGlobal.
  • hir-no-set-state-in-effect overlaps noDerivedStateEffect on the simplest shape; users can disable either via react-doctor.config.json.

Architecture summary

ESTree (oxlint visitor)
   ↓ lowerFunction
HIR (basic blocks + instructions + Place.originNode)
   ↓ inferTypes (per-identifier ReactType tags)
Typed HIR
   ↓ validateNo*  (per-rule analyses)
Findings (with originNode for precise reporting)
   ↓ runner reports through ruleContext
oxlint diagnostics anchored at the offending call site

Future ports (no/lazy-init-in-render, no-create-ref-during-render, deeper effect-event analyses) plug in as additional validators reading the same HIR.

Open in Web Open in Cursor 

@vercel
Copy link
Copy Markdown

vercel Bot commented May 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-doctor-website Ready Ready Preview, Comment May 8, 2026 9:25am

cursor Bot pushed a commit that referenced this pull request May 8, 2026
Five issues found on a fresh deep review of the HIR port (PR #164):

1. inferTypes conflated useState/useMemo/useContext returns under
   ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/
   StateSetter branch keyed off "Object", so a destructure like
   `const [n, runIt] = useMemo(...)` would tag `runIt` as a
   StateSetter and the validators would treat `runIt()` as a
   setState call. Fix: add a distinct `StateTuple` ReactType, gate
   the PropertyLoad branch on it. Regression test added.

2. The HIR rules duplicated `noDerivedStateEffect` on the canonical
   single-setter-call shape (article §1 fullName example), producing
   two diagnostics on the same line. Fix:
   `hir-no-derived-computations-in-effects` now defers when the
   effect body has no intermediate local bindings (StoreLocal in the
   inner HIR), preserving the multi-statement-with-locals path the
   AST walker can't see through. `hir-no-set-state-in-effect`
   stays unscoped because distinguishing direct vs aliased setter
   calls reliably from the HIR is not workable at v1; the duplicate
   on the simplest shape is documented as a known v1 limitation.

3. `lowerExpression` silently dropped `SpreadElement` arguments
   (`f(...args)` lowered to a CallExpression with the spread
   missing). Extracted `lowerCallArguments` that unwraps the
   spread's `argument` so the operand identity is still threaded
   through — losing only the spread shape (not modeled in v1) but
   keeping setState/alias propagation working through spread args.

4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement,
   ReturnStatement, BlockStatement, IfStatement, and the function-
   declaration shapes. `for`, `while`, `do-while`, `for-of`,
   `for-in`, `switch`, `try`, `throw`, `labeled` all silently
   fell through, so a useEffect (or any hook call) inside any of
   these blocks was invisible to validators. Added recursive
   descent for all of them — control flow collapses into the
   surrounding block (no real CFG terminals yet) but the bodies
   get lowered.

5. The hir-unit test for the article §1 example used the single-
   setter-call shape that the round-2 scoping fix now defers on;
   updated to the multi-statement-with-locals form to keep the
   coverage on the validator's unique path.

All 490 tests pass; lint, typecheck, format clean.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request May 8, 2026
Five issues found on a fresh deep review of the HIR port (PR #164):

1. inferTypes conflated useState/useMemo/useContext returns under
   ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/
   StateSetter branch keyed off "Object", so a destructure like
   `const [n, runIt] = useMemo(...)` would tag `runIt` as a
   StateSetter and the validators would treat `runIt()` as a
   setState call. Fix: add a distinct `StateTuple` ReactType, gate
   the PropertyLoad branch on it. Regression test added.

2. The HIR rules duplicated `noDerivedStateEffect` on the canonical
   single-setter-call shape (article §1 fullName example), producing
   two diagnostics on the same line. Fix:
   `hir-no-derived-computations-in-effects` now defers when the
   effect body has no intermediate local bindings (StoreLocal in the
   inner HIR), preserving the multi-statement-with-locals path the
   AST walker can't see through. `hir-no-set-state-in-effect`
   stays unscoped because distinguishing direct vs aliased setter
   calls reliably from the HIR is not workable at v1; the duplicate
   on the simplest shape is documented as a known v1 limitation.

3. `lowerExpression` silently dropped `SpreadElement` arguments
   (`f(...args)` lowered to a CallExpression with the spread
   missing). Extracted `lowerCallArguments` that unwraps the
   spread's `argument` so the operand identity is still threaded
   through — losing only the spread shape (not modeled in v1) but
   keeping setState/alias propagation working through spread args.

4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement,
   ReturnStatement, BlockStatement, IfStatement, and the function-
   declaration shapes. `for`, `while`, `do-while`, `for-of`,
   `for-in`, `switch`, `try`, `throw`, `labeled` all silently
   fell through, so a useEffect (or any hook call) inside any of
   these blocks was invisible to validators. Added recursive
   descent for all of them — control flow collapses into the
   surrounding block (no real CFG terminals yet) but the bodies
   get lowered.

5. The hir-unit test for the article §1 example used the single-
   setter-call shape that the round-2 scoping fix now defers on;
   updated to the multi-statement-with-locals form to keep the
   coverage on the validator's unique path.

All 490 tests pass; lint, typecheck, format clean.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
@cursor cursor Bot force-pushed the cursor/hir-port-7144 branch from c101b26 to 29c79d0 Compare May 8, 2026 09:03
cursor Bot pushed a commit that referenced this pull request May 8, 2026
Five issues found on a fresh deep review of the HIR port (PR #164):

1. inferTypes conflated useState/useMemo/useContext returns under
   ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/
   StateSetter branch keyed off "Object", so a destructure like
   `const [n, runIt] = useMemo(...)` would tag `runIt` as a
   StateSetter and the validators would treat `runIt()` as a
   setState call. Fix: add a distinct `StateTuple` ReactType, gate
   the PropertyLoad branch on it. Regression test added.

2. The HIR rules duplicated `noDerivedStateEffect` on the canonical
   single-setter-call shape (article §1 fullName example), producing
   two diagnostics on the same line. Fix:
   `hir-no-derived-computations-in-effects` now defers when the
   effect body has no intermediate local bindings (StoreLocal in the
   inner HIR), preserving the multi-statement-with-locals path the
   AST walker can't see through. `hir-no-set-state-in-effect`
   stays unscoped because distinguishing direct vs aliased setter
   calls reliably from the HIR is not workable at v1; the duplicate
   on the simplest shape is documented as a known v1 limitation.

3. `lowerExpression` silently dropped `SpreadElement` arguments
   (`f(...args)` lowered to a CallExpression with the spread
   missing). Extracted `lowerCallArguments` that unwraps the
   spread's `argument` so the operand identity is still threaded
   through — losing only the spread shape (not modeled in v1) but
   keeping setState/alias propagation working through spread args.

4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement,
   ReturnStatement, BlockStatement, IfStatement, and the function-
   declaration shapes. `for`, `while`, `do-while`, `for-of`,
   `for-in`, `switch`, `try`, `throw`, `labeled` all silently
   fell through, so a useEffect (or any hook call) inside any of
   these blocks was invisible to validators. Added recursive
   descent for all of them — control flow collapses into the
   surrounding block (no real CFG terminals yet) but the bodies
   get lowered.

5. The hir-unit test for the article §1 example used the single-
   setter-call shape that the round-2 scoping fix now defers on;
   updated to the multi-statement-with-locals form to keep the
   coverage on the validator's unique path.

All 490 tests pass; lint, typecheck, format clean.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
@cursor cursor Bot force-pushed the cursor/hir-port-7144 branch from 29c79d0 to e6c2397 Compare May 8, 2026 09:18
cursoragent and others added 16 commits May 8, 2026 09:23
Mirrors the structure of `babel-plugin-react-compiler/src/HIR/HIR.ts`
but heavily simplified for our needs:

  - operates on ESTree (oxlint plugin AST), not Babel
  - no SSA / phi nodes (uses a mutable name→Place table per-scope)
  - single block per function in v1 (no if/loop terminals modeled)
  - 13 instruction kinds (vs the compiler's 30+)

What's preserved:

  - data model: HIRFunction → blocks → instructions → places
  - lvalue / value-place / operand-place shape so validators can
    `switch (instr.value.kind)` exactly like the upstream code
  - identifier IDs are stable per binding, so propagation analyses
    (e.g. setState flowing through a const) work the same way
  - source locations threaded through every Place
  - type-tag layer (StateValue / StateSetter / UseEffectHook /
    RefValue / EffectEvent / PropCallback / …) and the matching
    isXType() predicates the upstream validators use

This commit is types-only; the lower / infer / validators / runner
land in subsequent commits to keep each layer reviewable on its own.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Walks the ESTree of a component and emits a single-block HIR with
SSA-flavored instructions. Mirrors the compiler's `BuildHIR.ts::lower`
shape (each AST expression becomes one or more Instructions whose
lvalue is a temporary Place; operands are Places that may load from
local bindings).

Key design points:

  - Shared id allocator across nested LoweringEnvironments so a
    captured outer binding keeps its IdentifierId when seen by an
    inner closure. This is what lets validators reason about
    'this effect callback closes over <X>' by Place identity rather
    than by name matching.
  - Parent-chain bindings lookup so `setCount` declared in the
    component body resolves to the same Place inside a useEffect
    callback.
  - ArrayPattern destructuring for `const [v, sv] = useState(...)`
    lowers to two indexed PropertyLoad instructions whose lvalues
    pick up StateValue / StateSetter types in the inferTypes pass.

Deliberate v1 simplifications:

  - control flow is flattened (if/while/for descend into their
    consequent/loop without producing terminal/branch instructions)
  - JSX collapses to a single Literal placeholder per element
  - effect annotations on Place all default to 'Read'; aliasing
    inference is out of scope for v1

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
inferTypes runs after lowering and tags Identifiers with React
semantic types (StateValue, StateSetter, UseEffectHook, RefValue,
EffectEvent, …). Implements a pared-down version of the compiler's
`inferTypes()` pass:

  - LoadGlobal of a known React hook name → tag the lvalue's type
  - CallExpression whose callee resolves to UseStateHook tags lvalue
    as Object (the [value, setter] pair); subsequent indexed
    PropertyLoad on it tags the destructured names as StateValue and
    StateSetter
  - PropertyLoad of `.current` on a RefValue → RefCurrent
  - LoadLocal / StoreLocal propagate the source identifier's type
    so `const fn = setState; fn(x)` is still seen as a setState
    call by the validators

printHIR emits a textual dump of an HIRFunction, used by the unit
test as a snapshot of the lower pass.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Two HIR-based validators, ported from the React Compiler:

  validateNoSetStateInEffects (~150 LOC, upstream is 347)
    Tracks setState propagation through LoadLocal, StoreLocal,
    FunctionExpression, and useEffectEvent wrappers — exactly the
    five instruction kinds the compiler's switch uses to seed its
    `setStateFunctions: Map<IdentifierId, Place>`. Reports when a
    useEffect's callback resolves (transitively) to a setState
    binding.

    v1 omissions vs upstream:
      - ref-derived setState exception (control-dominator analysis)
      - aliasing-effect tracking on operands

  validateNoDerivedComputationsInEffects (~120 LOC, upstream is 229)
    Three lookup tables that mirror the upstream code:
      candidateDependencies: lvalue → ArrayExpression instruction
      effectFunctions:       lvalue → FunctionExpression instruction
      localAliases:          lvalue → underlying source IdentifierId
    For `useEffect(arg0, arg1)` resolves arg0 to a tracked function
    and arg1 to a tracked deps array; reports when the inner function
    captures only those deps + setStates (= pure derivation, should
    move to render).

Both validators are pure functions of HIRFunction → finding[]; the
runner / oxlint Rule shell is in the next commit.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
…in runtime

Two thin oxlint Rules (`hir-no-set-state-in-effect`,
`hir-no-derived-computations-in-effects`) that:

  1. detect a component-shaped function (uppercase FunctionDeclaration
     or const Foo = () => ...)
  2. lower it to HIR and run inferTypes once
  3. forward validator findings via context.report()

Findings retain the React Compiler's diagnostic precision (Place
identity, type tags, source loc) but surface through the oxlint
plugin contract — no compiler runtime, no Babel, no second toolchain.

Registered in plugin/index.ts and oxlint-config.ts (severity: warn)
with category 'State & Effects' in run-oxlint.ts. Help text mentions
'Detected via HIR data flow analysis' so users can distinguish these
findings from the AST-walker rules. The two HIR rules deliberately
share IDs prefixed `hir-` so future ports stay namespaced.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Two test files:

  hir-port.test.ts — end-to-end through oxlint
    flag cases: setState-direct, setState-aliased (SSA propagation),
                 article §1 fullName derived-computation
    no-flag cases: setState inside subscription callback (sub-handler),
                   useEffect that reads non-dep prop (genuine sync)

  hir-unit.test.ts — direct calls into lower / infer / validators
    Bypasses oxlint to assert the HIR shape and validator findings on
    parsed source. Useful during development of the lower pass; logs
    a printHIR dump on failure so the IR is visible in CI output.

Adds @typescript-eslint/parser as a dev dep of the react-doctor
package (used by hir-unit.test.ts to parse source into ESTree).

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
The HIR unit test (`hir-unit.test.ts`) parses source via
@typescript-eslint/parser. Earlier in the lower-pass debugging
session a stray `pnpm add` planted the dep at the workspace root
too — that root entry was redundant with react-doctor's own devDep
and added needless monorepo surface. Drop it; the lockfile follows.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Add `originNode` to Place so diagnostics can anchor on the
offending source expression instead of the surrounding component.
Mirrors how the React Compiler threads `loc` through every Place,
but uses an actual node reference (not a separate location-to-node
table) since we already have the ESTree nodes in hand.

The lower pass now sets `originNode` for:
  - Identifier reads (LoadLocal / LoadGlobal)
  - Literal / TemplateLiteral
  - MemberExpression
  - CallExpression / MethodCall
  - ArrowFunctionExpression / FunctionExpression
  - ArrayExpression / ObjectExpression
  - BinaryExpression / LogicalExpression / UnaryExpression
  - ConditionalExpression
  - JSXElement / JSXFragment
  - VariableDeclarator (Identifier and ArrayPattern locals)
  - destructured prop entries
  - regular function parameters

Synthetic temporaries (e.g. method-call property places that don't
exist as a single AST node) get `originNode: null` and the runner
falls back to the component declaration.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
The validator's `findTopLevelSetStateCall` now returns BOTH the
original setter Place AND the call-site Place (the synthetic temp
whose `originNode` points at the offending CallExpression in source).
A parallel `innerCallSites` map propagates that call site through
`LoadLocal`/`StoreLocal`/`useEffectEvent` so a setState wrapped in
`useEffectEvent` still surfaces at the wrapper-call site, not at the
setter declaration site.

`SetStateInEffectFinding` gains a `callSitePlace` field; the runner
uses it in preference to `setterPlace` when reporting.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
…site

Two wiring improvements on top of the bare port:

  1. Per-component HIR cache (WeakMap keyed by the original ESTree
     node). When multiple HIR rules visit the same component during a
     single oxlint pass, lowering + inferTypes runs once instead of
     once-per-rule. The WeakMap lets the cache GC alongside the AST.

  2. Diagnostics anchor on the offending source expression. Each
     finding's Place carries an `originNode` (set during the lower
     pass). The runner uses it as the report node, falling back to
     the component declaration only for synthetic places that don't
     correspond to a single source node.

New regression test asserts the line number of the diagnostic for a
setState-in-effect violation: must be the line of the `setCount(1)`
call (line 6 in the fixture), not the line of the surrounding
`export const Counter = () => {}` declaration (line 3).

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
`Place.originNode` was typed `unknown | null`, which forced two
`as` casts in the runner (one for the WeakMap key, one when reading
back the originNode for context.report). Importing `EsTreeNode`
into hir/types.ts (no cycle — plugin/types.ts has no HIR imports)
lets us type the field as `EsTreeNode | null` directly. Both casts
gone; AGENTS.md 'Do not type cast unless absolutely necessary' is
honored.

Also adds the missing `isUseStateHookType` predicate. The type
`UseStateHook` was already in the `ReactType` union but had no
sibling helper alongside `isUseEffectHookType` /
`isUseRefType` etc.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
…ls) in validateEffect

The previous validateEffect walked every LoadLocal in the inner
function body and treated each one as a 'capture'. That conflated:

  - reads of variables captured from outer scope (real captures)
  - reads of variables LOCAL to the inner function (e.g. a temporary
    bound inside the effect body)

Mismatch in the second category caused inner-scope locals to be
flagged as 'captures-non-dep', which bailed the validator early and
silently dropped a true derived-computation finding. Reproduced by:

  useEffect(() => {
    const combined = firstName + ' ' + lastName;  // ← inner local
    setFullName(combined);
  }, [firstName, lastName]);                       // pure derivation

Fix: use the FunctionExpression instruction's `capturedPlaces` field
(populated by lower.ts's `collectCapturedPlaces`). That set is
exactly the outer bindings the inner reads — the 1:1 analog of the
upstream compiler's `effectFunction.context`.

The `effectFunctions` lookup table now stores both the HIRFunction
and its capture list; the validator iterates only the captures.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
…rEnv parameter

Two cleanups in lower.ts:

  1. `lowerExpression` now also handles nested FunctionDeclaration.
     Previously a `function helper() {}` inside a component body
     fell through to the Unsupported case AND its name wasn't bound
     in the enclosing env, so a subsequent `helper()` call would
     LoadGlobal the name instead of resolving to the inner function.
     We lower the FunctionDeclaration like an Arrow/FunctionExpression
     and tie its name to the lvalue Place when entering the binding.

  2. `collectCapturedPlaces` had a dead `outerEnv` parameter the
     comment apologized for with `void outerEnv`. The function only
     ever needed the inner HIRFunction (Place identifier ids are
     already shared via the root-env id allocator, so an outer-env
     argument was never load-bearing). Drop it.

Test cleanup: `hir-unit.test.ts` no longer uses `as any` to coerce
the parser output; it does a single explicit `as unknown as
EsTreeNode` with a HACK comment explaining why the parser's
Program.body[0] is structurally compatible with our EsTreeNode
interface.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Five issues found on a fresh deep review of the HIR port (PR #164):

1. inferTypes conflated useState/useMemo/useContext returns under
   ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/
   StateSetter branch keyed off "Object", so a destructure like
   `const [n, runIt] = useMemo(...)` would tag `runIt` as a
   StateSetter and the validators would treat `runIt()` as a
   setState call. Fix: add a distinct `StateTuple` ReactType, gate
   the PropertyLoad branch on it. Regression test added.

2. The HIR rules duplicated `noDerivedStateEffect` on the canonical
   single-setter-call shape (article §1 fullName example), producing
   two diagnostics on the same line. Fix:
   `hir-no-derived-computations-in-effects` now defers when the
   effect body has no intermediate local bindings (StoreLocal in the
   inner HIR), preserving the multi-statement-with-locals path the
   AST walker can't see through. `hir-no-set-state-in-effect`
   stays unscoped because distinguishing direct vs aliased setter
   calls reliably from the HIR is not workable at v1; the duplicate
   on the simplest shape is documented as a known v1 limitation.

3. `lowerExpression` silently dropped `SpreadElement` arguments
   (`f(...args)` lowered to a CallExpression with the spread
   missing). Extracted `lowerCallArguments` that unwraps the
   spread's `argument` so the operand identity is still threaded
   through — losing only the spread shape (not modeled in v1) but
   keeping setState/alias propagation working through spread args.

4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement,
   ReturnStatement, BlockStatement, IfStatement, and the function-
   declaration shapes. `for`, `while`, `do-while`, `for-of`,
   `for-in`, `switch`, `try`, `throw`, `labeled` all silently
   fell through, so a useEffect (or any hook call) inside any of
   these blocks was invisible to validators. Added recursive
   descent for all of them — control flow collapses into the
   surrounding block (no real CFG terminals yet) but the bodies
   get lowered.

5. The hir-unit test for the article §1 example used the single-
   setter-call shape that the round-2 scoping fix now defers on;
   updated to the multi-statement-with-locals form to keep the
   coverage on the validator's unique path.

All 490 tests pass; lint, typecheck, format clean.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Two follow-up cleanups on the round-5 HIR review:

1. `TryStatement` lowering didn't bind the catch parameter
   (`} catch (error) {`), so a reference to `error` inside the
   handler body fell through to LoadGlobal — wrong for any
   future validator that traces identifier reads through try/catch.
   Bind the param's Identifier in the surrounding env (no real
   block scoping in v1; minor over-scope is acceptable).

2. Dead loop at the top of `inferTypes` that walked
   `fn.destructuredProps` and did nothing on every entry —
   removed. The intent (tag function-typed props) is already
   covered by lower.ts's `on[A-Z]` heuristic.

All 490 tests pass; lint, typecheck, format clean.

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Cleanup pass following round-5 review. No behavior changes — all 490
tests pass, lint/typecheck/format clean.

Removed:
- types.ts: unused predicates (isStateValueType, isUseStateHookType,
  isUseRefType, isRefValueType, isEffectEventType, isPropCallbackType),
  unused 'prop' Identifier.origin variant, unused EffectKind union
  members (Mutate / Capture / Store / Freeze).
- print.ts: entire 80-line file (printHIR was only referenced by
  hir-unit.test.ts's debug logging, which is also removed).
- validate-no-derived-computations-in-effects.ts: dead 'reason'
  field on findings — only one value was ever consumed by the
  runner; collapsed validator to return null on the other paths.
- hir-unit.test.ts: console.log + JSON.stringify debug output that
  spammed CI on every run; replaced with proper expect()s.
- 200+ lines of doc-style comments across types.ts / lower.ts /
  infer-types.ts / runner.ts / validators / hir-port.test.ts that
  re-explained what the code says or repeated information from the
  PR description.

Kept (with concise // HACK: prefix):
- StateTuple discriminator (real semantic distinction)
- SpreadElement unwrap (real ESTree wart)
- Catch-param binding (real correctness fix)
- Multi-statement-with-locals defer (real overlap mitigation)
- Inner-call-site Place tracking (real diagnostic-anchor concern)

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
@cursor cursor Bot force-pushed the cursor/hir-port-7144 branch from e6c2397 to e6211a0 Compare May 8, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants